feat(vnext): complete SQL editor overhaul - #202
Merged
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 61 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Member
Author
|
Addressed all 27 review threads in Highlights:
Two final adversarial reviews found seven additional cross-layer issues; those Final local verification:
|
There was a problem hiding this comment.
2 issues found across 29 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/vnext/column-query-site.ts">
<violation number="1" location="src/vnext/column-query-site.ts:538">
P2: Column completion inside PostgreSQL `LATERAL` subqueries omits preceding outer relations while reporting complete coverage. The new `FROM` exclusion treats LATERAL like an ordinary derived table; distinguish LATERAL and collect only preceding parent relations (or mark this scope partial/unavailable).</violation>
</file>
<file name="src/vnext/node-sql-parser-query-bindings.ts">
<violation number="1" location="src/vnext/node-sql-parser-query-bindings.ts:752">
P1: Recursive CTE bodies can be completed as physical tables: this visits the CTE body before its own declaration is visible, so `FROM x` in `WITH RECURSIVE x AS (...)` becomes a complete `named` binding. Model recursive visibility explicitly, or downgrade that binding/coverage with `recursive-cte-uncertainty` rather than publishing false relation evidence.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/vnext/__tests__/session.test.ts">
<violation number="1" location="src/vnext/__tests__/session.test.ts:603">
P2: Adding shouldAdvanceTime: true makes performance.now() auto-advance based on real wall clock time, which can introduce flakiness into tests that rely on precise fake-timer timing for budget calculations. The default vi.useFakeTimers() already gives full control over time advancement.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Completes the final batched vNext SQL language-service overhaul milestone.
node-sql-parsernormalization in the isolated worker protocolprovenance, epochs, bounded caches, cancellation, partial/loading/failure
states, and hostile-response validation
sessions and the CodeMirror adapter
for embedded marimo expressions while preserving external sources
isolation
documentation
Review hardening
All 27 Cubic comments were reproduced and addressed. Two final adversarial
reviews found and closed seven additional issues:
identity comparisons, and blank namespace path components
including reentrant abort listeners
blocks, and
ON/USINGvisibilitydeterministic partial cap, incomplete aliases, and line-comment boundaries
failure-safe browser cleanup
CTE and derived-table output-column inference remains an explicit next-major
cutover gap; vNext does not incorrectly route those logical relations to the
physical catalog provider.
Correctness and performance
6.4 ms
within the unchanged 184 KiB raw and 50 KiB gzip limits
Verification
statements/branches/functions/lines gate
isolated-worker placement audit pass
Advances the next-major implementation milestone tracked in #169.